Skip to content

Fix JsonSchemaExporter dropping nullability for nullable floating-point composition schemas#129530

Merged
eiriktsarpalis merged 13 commits into
mainfrom
copilot/fix-jsonschemaexporter-null-branch
Jun 22, 2026
Merged

Fix JsonSchemaExporter dropping nullability for nullable floating-point composition schemas#129530
eiriktsarpalis merged 13 commits into
mainfrom
copilot/fix-jsonschemaexporter-null-branch

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

JsonSchemaExporter omitted null-acceptance for CLR-nullable types (double?, float?, Half?) whose schema is emitted as an anyOf composition — which happens for IEEE floating-point types under JsonNumberHandling.AllowNamedFloatingPointLiterals. The same property exports correctly as {"type":["number","null"]} under default number handling, so a double? was advertised to downstream JSON-Schema/OpenAPI consumers as a non-nullable number.

Description

  • Root causeJsonSchema.MakeNullable only adds JsonSchemaType.Null when the schema carries a concrete type keyword. Composition schemas (anyOf) have Type == Any, so nullable wrappers over floating-point anyOf schemas could lose nullability.
  • Fix — Introduced a new internal virtual bool IsIeeeFloatingPointConverter property on JsonConverter, overridden to true in DoubleConverter, SingleConverter, and HalfConverter. Updated the nullable-value-type path in JsonSchemaExporter.MapJsonSchemaCore to check both this property and the presence of AllowNamedFloatingPointLiterals in the effective number handling. When both conditions are met, null is folded into the numeric branch's type keyword ("type": ["number","null"]). Added Debug.Assert statements to validate the expected anyOf schema shape for IEEE floating-point types with AllowNamedFloatingPointLiterals.
  • Tests — Added regression coverage for double?, float?, Half?, and a POCO with nullable floating-point properties under global AllowNamedFloatingPointLiterals, across reflection and source-gen test contexts.

Resulting export for a nullable property:

// double? under AllowNamedFloatingPointLiterals
{"anyOf":[{"type":["number","null"]},{"enum":["NaN","Infinity","-Infinity"]}]}

Copilot AI requested review from Copilot and removed request for Copilot June 17, 2026 16:32
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 17, 2026 17:06
Copilot AI changed the title [WIP] Fix JsonSchemaExporter omitting null branch for nullable numbers Fix JsonSchemaExporter dropping null branch for nullable anyOf composition schemas Jun 17, 2026
Copilot AI requested a review from eiriktsarpalis June 17, 2026 17:07
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

@eiriktsarpalis eiriktsarpalis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot This doesn't look right.

Comment thread src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchema.cs Outdated
Comment thread src/libraries/System.Text.Json/tests/Common/JsonSchemaExporterTests.TestTypes.cs Outdated
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 18, 2026 13:22

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot This doesn't look right.

Agreed — the original approach was wrong. I've reworked it in 3279084a: instead of appending a standalone { "type": "null" } branch to the anyOf, MakeNullable now folds null into the first branch that carries a concrete type. So a double? under AllowNamedFloatingPointLiterals now exports as:

{"anyOf":[{"type":["number","null"]},{"enum":["NaN","Infinity","-Infinity"]}]}

This also handles the empty/type-less anyOf edge case (no spurious null branch) and skips branches that already permit null. The test expectations were corrected accordingly. (Note: my two earlier thread replies referenced an incorrect short hash; the correct commit is 3279084a.)

Comment thread src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchema.cs Outdated
…, revert MakeNullable

Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 18, 2026 16:01
Copilot AI changed the title Fix JsonSchemaExporter dropping null branch for nullable anyOf composition schemas Fix JsonSchemaExporter dropping null branch for nullable floating-point composition schemas Jun 18, 2026

@eiriktsarpalis eiriktsarpalis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs Outdated
Comment thread src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs Outdated
Co-authored-by: eiriktsarpalis <10833894+eiriktsarpalis@users.noreply.github.com>
Introduced a virtual property `IsIeeeFloatingPointConverter` on `JsonConverter`, overridden to `true` in `DoubleConverter`, `SingleConverter`, and `HalfConverter`. Updated `JsonSchemaExporter` to check this property instead of performing brittle element type checks, preventing false positives from nullable struct unions. Replaced the boolean foundNumberBranch with a Debug.Assert that checks the list before iteration. Added test coverage for nullable struct unions to ensure nullability is preserved without affecting IEEE floating-point schema generation.

Co-authored-by: eiriktsarpalis <16040868+eiriktsarpalis@users.noreply.github.com>
Copilot AI requested a review from jozkee June 19, 2026 17:36
Comment thread src/libraries/System.Text.Json/tests/Common/JsonSchemaExporterTests.TestTypes.cs Outdated
Replaced the IStructDiscriminatedUnion polymorphic interface test with a proper C# union type (MyUnion(int, string)) as requested. The union type test validates that nullable struct unions preserve nullability in anyOf schemas without relying on the IEEE floating-point converter path.

Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
Comment thread src/libraries/System.Text.Json/src/System/Text/Json/Schema/JsonSchemaExporter.cs Outdated
…E float check

Following review feedback, the nullable anyOf handling now:
1. Only executes for IEEE floating-point converters with AllowNamedFloatingPointLiterals
2. Asserts that anyOf schema exists (as it always should for this specific case)
3. Removes the unreachable else branch that was never hit by any test case

The MyUnion? test case and type declaration have been removed because union schema generation handles nullable cases internally via caseInfo.IsNullable, so the nullable wrapper code path is never exercised for unions.

Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
@eiriktsarpalis eiriktsarpalis enabled auto-merge (squash) June 22, 2026 16:17
@eiriktsarpalis eiriktsarpalis merged commit 5721593 into main Jun 22, 2026
83 of 88 checks passed
@eiriktsarpalis eiriktsarpalis deleted the copilot/fix-jsonschemaexporter-null-branch branch June 22, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants